Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TIR] Fix reverse_compute_at for trivial region with trivial block var #11234

Merged
merged 2 commits into from
May 8, 2022
Merged

[TIR] Fix reverse_compute_at for trivial region with trivial block var #11234

merged 2 commits into from
May 8, 2022

Conversation

vinx13
Copy link
Member

@vinx13 vinx13 commented May 6, 2022

This PR fixed compute_at / reverse_compute_at when block access regions contain constants (e.g. A[0, vi, vj]).
Background:
#11187 added additional simplification when creating prim func from TE. This may result in block var with constant binding values simplified.

cc @junrushao1994 @spectrometerHBH @zxybazh @shingjan

shingjan
shingjan approved these changes May 6, 2022
View reviewed changes
Copy link

@shingjan shingjan left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can confirm that this PR fixed my repro on meta_schedule with LLVM. Thanks! @vinx13. BTW do you have any insight on why this issue is only happening on LLVM but not CUDA?

@vinx13
Copy link
Member Author

vinx13 commented May 6, 2022

CUDA will call cache_write to introduce a cache block first, so the block used for reverse_compute_at is different, and the bindings we are dealing with might also be different

zxybazh
zxybazh approved these changes May 6, 2022
View reviewed changes
Copy link
Member

@zxybazh zxybazh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the quick fix and thanks to @shingjan for reporting this issue!

@vinx13 vinx13 force-pushed the fix/rev_compute_trivial branch from 5fba48d to 42231b8 Compare May 6, 2022 22:22
@junrushao
Copy link
Member

Reading the cryptic error message, it seems that the IntervalSet sometimes is used to represent handle vs int, so it cans CanProveEqual - a bit weird to me, but definitely fixable with more guards

  16: _M_invoke
        at /usr/include/c++/7/bits/std_function.h:316
  15: operator()
        at /workspace/src/te/operation/compute_op.cc:216
  14: tvm::arith::IntSetAnalyzer::operator()(tvm::PrimExpr const&, tvm::runtime::Map<tvm::tir::Var, tvm::arith::IntSet, void, void> const&)
        at /workspace/src/arith/int_set.cc:521
  13: tvm::arith::IntSetAnalyzer::Impl::Eval(tvm::PrimExpr const&, tvm::runtime::Map<tvm::tir::Var, tvm::arith::IntSet, void, void> const&) const
        at /workspace/src/arith/int_set.cc:509
  12: tvm::arith::IntervalSetEvaluator::Eval(tvm::PrimExpr const&)
        at /workspace/src/arith/int_set.cc:351
  11: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::VisitExpr(tvm::PrimExpr const&)
        at /workspace/include/tvm/tir/expr_functor.h:114
  10: tvm::NodeFunctor<tvm::arith::IntervalSet (tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)>::operator()(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*) const
        at /workspace/include/tvm/node/functor.h:97
  9: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::InitVTable()::{lambda(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)#8}::_FUN(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)
        at /workspace/include/tvm/tir/expr_functor.h:171
  8: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::InitVTable()::{lambda(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)#8}::operator()(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*) const
        at /workspace/include/tvm/tir/expr_functor.h:171
  7: tvm::arith::IntervalSetEvaluator::VisitExpr_(tvm::tir::AddNode const*)
        at /workspace/src/arith/int_set.cc:383
  6: tvm::arith::IntervalSet tvm::arith::IntervalSetEvaluator::VisitBinaryExpr_<tvm::tir::Add, tvm::tir::AddNode>(tvm::tir::AddNode const*)
        at /workspace/src/arith/int_set.cc:493
  5: tvm::arith::IntervalSet tvm::arith::Combine<tvm::tir::Add>(tvm::arith::Analyzer*, tvm::arith::IntervalSet, tvm::arith::IntervalSet)
        at /workspace/src/arith/int_set.cc:126
  4: tvm::arith::IntervalSetNode::IsSinglePoint() const
        at /workspace/src/arith/interval_set.h:67
  3: tvm::arith::Analyzer::CanProveEqual(tvm::PrimExpr const&, tvm::PrimExpr const&)
        at /workspace/src/arith/analyzer.cc:107
  2: tvm::operator==(tvm::PrimExpr, tvm::PrimExpr)
        at /workspace/src/tir/op/op.cc:501
  1: tvm::equal(tvm::PrimExpr, tvm::PrimExpr, tvm::Span)
        at /workspace/src/tir/op/op.cc:503
  0: tvm::BinaryOpMatchTypes(tvm::PrimExpr&, tvm::PrimExpr&, tvm::Span)
        at /workspace/src/tir/op/op.cc:163
  File "/workspace/src/tir/op/op.cc", line 163
TVMError: Cannot match type handle vs int32

@wrongtest-intellif
Copy link
Contributor

Reading the cryptic error message, it seems that the IntervalSet sometimes is used to represent handle vs int, so it cans CanProveEqual - a bit weird to me, but definitely fixable with more guards

In IntervalSet, positive and negative infinity are two special values with type handle, and intset.min() and intset.max() should check whether be infinity, can not directly assume they are of integer type.

@junrushao
Copy link
Member

junrushao commented May 7, 2022 •
edited
Loading

That makes a lot of sense @wrongtest

@Hzfengsy Hzfengsy merged commit d049079 into apache:main May 8, 2022
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
@vinx13
[TIR] Fix reverse_compute_at for trivial region with trivial block var (
eb09860 
apache#11234)

* [TIR] Fix reverse_compute_at for trivial region with trivial block var

* Prevent handle arithmetics
SebastianBoblest pushed a commit to SebastianBoblest/tvm that referenced this pull request May 27, 2022
@vinx13
[TIR] Fix reverse_compute_at for trivial region with trivial block var (
a68e10b 
apache#11234)

* [TIR] Fix reverse_compute_at for trivial region with trivial block var

* Prevent handle arithmetics
@tqchen tqchen mentioned this pull request Apr 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hzfengsy Hzfengsy Hzfengsy approved these changes

@shingjan shingjan shingjan approved these changes

@zxybazh zxybazh zxybazh approved these changes

@junrushao junrushao Awaiting requested review from junrushao

@spectrometerHBH spectrometerHBH Awaiting requested review from spectrometerHBH

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vinx13 @junrushao @wrongtest-intellif @zxybazh @shingjan @Hzfengsy